-
Notifications
You must be signed in to change notification settings - Fork 19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Addressing Breaking Change to Dates #56
Conversation
src/Missings.jl
Outdated
@@ -2,6 +2,9 @@ __precompile__(true) | |||
module Missings | |||
|
|||
import Base: *, <, ==, !=, <=, !, +, -, ^, /, &, |, xor | |||
if VERSION >= v"0.7.0-DEV.2575" | |||
import Dates |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess using
should be enough, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should, but import
is safer since Base.Dates
used to export only certain names. I believe they should be the same now that it moved to stdlib.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it's not safer, as it allows overriding functions in Dates
, which is not what we want to do AFAIK.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It depends on the style. using
has no overriding allowed, but floods the namespace (especially with packages that export many functions and structs) and can cause issues if multiple packages have the same functions. import
allows to override (though you still have to do it explicitly), but keeps the namespace clear. There is also a warning if any package overrides a method. I always use the module prefix (except with Base
) since I saw it in an old Julia blog. Either one would do and probably Compat will probably check for the optimal solution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
using
also allows specifying which objects you want to use.
Thanks. I think we'd better wait for Compat to support this, though. |
Codecov Report
@@ Coverage Diff @@
## master #56 +/- ##
=======================================
Coverage 94.03% 94.03%
=======================================
Files 1 1
Lines 151 151
=======================================
Hits 142 142
Misses 9 9
Continue to review full report at Codecov.
|
Addressing [0c0d6cc](JuliaLang/julia@0c0d6cc) If Nulls is getting a version for Julia 0.7 it would need a similar fix (until Compat?).
Thanks! |
Addressing 0c0d6cc
If Nulls is getting a version for Julia 0.7 it would need a similar fix (until Compat?).